screencastService: Propagate machine-parseable error types to gnome-shell

We'll be using hardware encoding for screencasts soon, so we'll likely see
more things go wrong in the future, including crashes of the whole
screencastService. To deal with this, we'll introduce logic to blocklist
certain recording pipelines in case of failure and also add some logic
to retry the recording automatically.

To allow for better messaging to the user in those failure cases, we want
to be aware in gnome-shell, what exactly the error in the recorder was.

So propagate the most common types of errors that can happen in the
ScreencastService to gnome-shell using the new DBusError module.

Part-of: <https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2976>

diff --git a/data/dbus-interfaces/org.gnome.Shell.Screencast.xml b/data/dbus-interfaces/org.gnome.Shell.Screencast.xml
index 6512396d94674822f36e055e60f790b72ae9a086..9a8eb329355c80a844208d7ffc1c4a33428d42d7 100644 (file)
--- a/data/dbus-interfaces/org.gnome.Shell.Screencast.xml
+++ b/data/dbus-interfaces/org.gnome.Shell.Screencast.xml
@@ -93,6 +93,7 @@
     </method>
 
     <signal name="Error">
+      <arg type="s" name="name"/>
       <arg type="s" name="message"/>
     </signal>
 

diff --git a/js/dbusServices/screencast/screencastService.js b/js/dbusServices/screencast/screencastService.js
index 9c69af9cf5f03b02b34b813b7271ea725a1a2d30..b87443e08d12749618e818e6e06e77db9dbd8c98 100644 (file)
--- a/js/dbusServices/screencast/screencastService.js
+++ b/js/dbusServices/screencast/screencastService.js
@@ -6,6 +6,7 @@ import Gtk from 'gi://Gtk?version=4.0';
 
 import {ServiceImplementation} from './dbusService.js';
 
+import {ScreencastErrors, ScreencastError} from './misc/dbusErrors.js';
 import {loadInterfaceXML, loadSubInterfaceXML} from './misc/dbusUtils.js';
 import * as Signals from './misc/signals.js';
 
@@ -150,13 +151,13 @@ class Recorder extends Signals.EventEmitter {
         }
     }
 
-    _bailOutOnError(error) {
+    _bailOutOnError(message, errorDomain = ScreencastErrors, errorCode = ScreencastError.RECORDER_ERROR) {
+        const error = new GLib.Error(errorDomain, errorCode, message);
+
         this._teardownPipeline();
         this._unwatchSender();
         this._stopSession();
 
-        log(`Recorder error: ${error.message}`);
-
         if (this._startRequest) {
             this._startRequest.reject(error);
             delete this._startRequest;
@@ -170,13 +171,13 @@ class Recorder extends Signals.EventEmitter {
         this.emit('error', error);
     }
 
-    _handleFatalPipelineError(message) {
+    _handleFatalPipelineError(message, errorDomain, errorCode) {
         this._pipelineState = PipelineState.ERROR;
-        this._bailOutOnError(new Error(`Fatal pipeline error: ${message}`));
+        this._bailOutOnError(message, errorDomain, errorCode);
     }
 
     _senderVanished() {
-        this._bailOutOnError(new Error('Sender has vanished'));
+        this._bailOutOnError('Sender has vanished');
     }
 
     _onSessionClosed() {
@@ -184,7 +185,7 @@ class Recorder extends Signals.EventEmitter {
             return; // We closed the session ourselves
 
         this._sessionState = SessionState.STOPPED;
-        this._bailOutOnError(new Error('Session closed unexpectedly'));
+        this._bailOutOnError('Session closed unexpectedly');
     }
 
     _initSession(sessionPath) {
@@ -197,7 +198,8 @@ class Recorder extends Signals.EventEmitter {
     _tryNextPipeline() {
         const {done, value: pipelineConfig} = this._pipelineConfigs.next();
         if (done) {
-            this._handleFatalPipelineError('All pipelines failed to start');
+            this._handleFatalPipelineError('All pipelines failed to start',
+                ScreencastErrors, ScreencastError.ALL_PIPELINES_FAILED);
             return;
         }
 
@@ -325,7 +327,8 @@ class Recorder extends Signals.EventEmitter {
 
             case PipelineState.PLAYING:
                 this._addRecentItem();
-                this._handleFatalPipelineError('Unexpected EOS message');
+                this._handleFatalPipelineError('Unexpected EOS message',
+                    ScreencastErrors, ScreencastError.PIPELINE_ERROR);
                 break;
 
             case PipelineState.FLUSHING:
@@ -358,11 +361,14 @@ class Recorder extends Signals.EventEmitter {
                 break;
 
             case PipelineState.PLAYING:
-            case PipelineState.FLUSHING:
-                // Everything else we can't handle, so error out
+            case PipelineState.FLUSHING: {
+                const [error] = message.parse_error();
                 this._handleFatalPipelineError(
-                    `GStreamer error while in state ${this._pipelineState}: ${message.parse_error()[0].message}`);
+                    `GStreamer error while in state ${this._pipelineState}: ${error.message}`,
+                    ScreencastErrors, ScreencastError.PIPELINE_ERROR);
+
                 break;
+            }
 
             default:
                 break;
@@ -526,17 +532,19 @@ export const ScreencastService = class extends ServiceImplementation {
     }
 
     async ScreencastAsync(params, invocation) {
-        let returnValue = [false, ''];
-
         if (this._lockdownSettings.get_boolean('disable-save-to-disk')) {
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            invocation.return_error_literal(ScreencastErrors,
+                ScreencastError.SAVE_TO_DISK_DISABLED,
+                'Saving to disk is disabled');
             return;
         }
 
         const sender = invocation.get_sender();
 
         if (this._recorders.get(sender)) {
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            invocation.return_error_literal(ScreencastErrors,
+                ScreencastError.ALREADY_RECORDING,
+                'Service is already recording');
             return;
         }
 
@@ -558,7 +566,10 @@ export const ScreencastService = class extends ServiceImplementation {
                 invocation);
         } catch (error) {
             log(`Failed to create recorder: ${error.message}`);
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            invocation.return_error_literal(ScreencastErrors,
+                ScreencastError.RECORDER_ERROR,
+                error.message);
+
             return;
         }
 
@@ -566,33 +577,46 @@ export const ScreencastService = class extends ServiceImplementation {
 
         try {
             await recorder.startRecording();
-            returnValue = [true, filePath];
+            invocation.return_value(GLib.Variant.new('(bs)', [true, filePath]));
         } catch (error) {
             log(`Failed to start recorder: ${error.message}`);
             this._removeRecorder(sender);
-        } finally {
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            if (error instanceof GLib.Error) {
+                invocation.return_gerror(error);
+            } else {
+                invocation.return_error_literal(ScreencastErrors,
+                    ScreencastError.RECORDER_ERROR,
+                    error.message);
+            }
+
+            return;
         }
 
         recorder.connect('error', (r, error) => {
+            log(`Fatal error while recording: ${error.message}`);
             this._removeRecorder(sender);
             this._dbusImpl.emit_signal('Error',
-                new GLib.Variant('(s)', [error.message]));
+                new GLib.Variant('(ss)', [
+                    Gio.DBusError.encode_gerror(error),
+                    error.message,
+                ]));
         });
     }
 
     async ScreencastAreaAsync(params, invocation) {
-        let returnValue = [false, ''];
-
         if (this._lockdownSettings.get_boolean('disable-save-to-disk')) {
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            invocation.return_error_literal(ScreencastErrors,
+                ScreencastError.SAVE_TO_DISK_DISABLED,
+                'Saving to disk is disabled');
             return;
         }
 
         const sender = invocation.get_sender();
 
         if (this._recorders.get(sender)) {
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            invocation.return_error_literal(ScreencastErrors,
+                ScreencastError.ALREADY_RECORDING,
+                'Service is already recording');
             return;
         }
 
@@ -613,7 +637,10 @@ export const ScreencastService = class extends ServiceImplementation {
                 invocation);
         } catch (error) {
             log(`Failed to create recorder: ${error.message}`);
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            invocation.return_error_literal(ScreencastErrors,
+                ScreencastError.RECORDER_ERROR,
+                error.message);
+
             return;
         }
 
@@ -621,18 +648,29 @@ export const ScreencastService = class extends ServiceImplementation {
 
         try {
             await recorder.startRecording();
-            returnValue = [true, filePath];
+            invocation.return_value(GLib.Variant.new('(bs)', [true, filePath]));
         } catch (error) {
             log(`Failed to start recorder: ${error.message}`);
             this._removeRecorder(sender);
-        } finally {
-            invocation.return_value(GLib.Variant.new('(bs)', returnValue));
+            if (error instanceof GLib.Error) {
+                invocation.return_gerror(error);
+            } else {
+                invocation.return_error_literal(ScreencastErrors,
+                    ScreencastError.RECORDER_ERROR,
+                    error.message);
+            }
+
+            return;
         }
 
         recorder.connect('error', (r, error) => {
+            log(`Fatal error while recording: ${error.message}`);
             this._removeRecorder(sender);
             this._dbusImpl.emit_signal('Error',
-                new GLib.Variant('(s)', [error.message]));
+                new GLib.Variant('(ss)', [
+                    Gio.DBusError.encode_gerror(error),
+                    error.message,
+                ]));
         });
     }
 

diff --git a/js/misc/dbusErrors.js b/js/misc/dbusErrors.js
index a00288d2ced1cabfef46433bdeb945358b973883..ff84f31402872cf961cb32f2439dbb677f87953c 100644 (file)
--- a/js/misc/dbusErrors.js
+++ b/js/misc/dbusErrors.js
@@ -43,3 +43,14 @@ export const ExtensionError = {
 };
 export const ExtensionErrors =
     registerErrorDomain('Extensions', ExtensionError);
+
+export const ScreencastError = {
+    ALL_PIPELINES_FAILED: 0,
+    PIPELINE_ERROR: 1,
+    SAVE_TO_DISK_DISABLED: 2,
+    ALREADY_RECORDING: 3,
+    RECORDER_ERROR: 4,
+    SERVICE_CRASH: 5,
+};
+export const ScreencastErrors =
+    registerErrorDomain('Screencast', ScreencastError);

diff --git a/js/ui/screenshot.js b/js/ui/screenshot.js
index f204d383df07dcb78816ac29e5486921177e0cb4..e60f1c5699f91644719bafe8f4ce055dd5c02fcd 100644 (file)
--- a/js/ui/screenshot.js
+++ b/js/ui/screenshot.js
@@ -25,6 +25,7 @@ Gio._promisify(Shell.Screenshot.prototype, 'screenshot_area');
 Gio._promisify(Shell.Screenshot.prototype, 'screenshot_stage_to_content');
 Gio._promisify(Shell.Screenshot, 'composite_to_stream');
 
+import {ScreencastErrors, ScreencastError} from '../misc/dbusErrors.js';
 import {loadInterfaceXML} from '../misc/fileUtils.js';
 import {DBusSenderChecker} from '../misc/util.js';
 
@@ -1099,8 +1100,8 @@ export const ScreenshotUI = GObject.registerClass({
                 this._syncCastButton();
             });
 
-        this._screencastProxy.connectSignal('Error',
-            () => this._screencastFailed());
+        this._screencastProxy.connectSignal('Error', (proxy, sender, [errorName, message]) =>
+            this._screencastFailed(Gio.DBusError.new_for_dbus_error(errorName, message)));
 
         this._screencastProxy.connect('notify::g-name-owner', () => {
             if (this._screencastProxy.g_name_owner)
@@ -1109,7 +1110,9 @@ export const ScreenshotUI = GObject.registerClass({
             if (!this._screencastInProgress)
                 return;
 
-            this._screencastFailed();
+            this._screencastFailed(
+                new GLib.Error(ScreencastErrors, ScreencastError.SERVICE_CRASH,
+                    'Service crashed'));
         });
 
         this._lockdownSettings = new Gio.Settings({schema_id: 'org.gnome.desktop.lockdown'});
@@ -1974,7 +1977,7 @@ export const ScreenshotUI = GObject.registerClass({
         this._setScreencastInProgress(true);
 
         try {
-            const [success, path] = await method(
+            const [, path] = await method(
                 GLib.build_filenamev([
                     /* Translators: this is the folder where recorded
                        screencasts are stored. */
@@ -1986,8 +1989,7 @@ export const ScreenshotUI = GObject.registerClass({
                     _('Screencast from %d %t.webm'),
                 ]),
                 {'draw-cursor': new GLib.Variant('b', drawCursor)});
-            if (!success)
-                throw new Error();
+
             this._screencastPath = path;
         } catch (error) {
             this._setScreencastInProgress(false);
@@ -2024,7 +2026,9 @@ export const ScreenshotUI = GObject.registerClass({
         this._showNotification(_('Screencast recorded'));
     }
 
-    _screencastFailed() {
+    _screencastFailed(error) {
+        console.error(`Screencast failed: ${error}`);
+
         this._setScreencastInProgress(false);
 
         // Translators: notification title.